Add Claude Code provider integration with provider-aware auth and rich-content support#563
Add Claude Code provider integration with provider-aware auth and rich-content support#563blas0 wants to merge 2 commits intopingdotgg:mainfrom
Conversation
# Conflicts: # apps/web/src/components/ChatMarkdown.tsx # apps/web/src/components/ChatView.tsx # apps/web/src/session-logic.ts
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }); | ||
|
|
||
| const runtimeEventQueue = yield* Queue.unbounded<ProviderRuntimeEvent>(); | ||
|
|
There was a problem hiding this comment.
🟢 Low Layers/ClaudeCodeAdapter.ts:540
The event listener forks a concurrent Effect for each incoming event and awaits writeNativeEvent(event) before enqueuing to runtimeEventQueue. Because the manager emits lifecycle events (session/started, thread/started, etc.) synchronously and file I/O timing is non-deterministic, a later event can finish logging and reach the queue before an earlier one. This violates the strict causal ordering required by the protocol and will cause downstream failures.
Consider enqueuing events immediately to preserve order, then performing logging downstream (e.g., using Stream.tap on the consumer side) or by serializing the ingestion path.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ClaudeCodeAdapter.ts around line 540:
The event listener forks a concurrent `Effect` for each incoming event and awaits `writeNativeEvent(event)` before enqueuing to `runtimeEventQueue`. Because the manager emits lifecycle events (`session/started`, `thread/started`, etc.) synchronously and file I/O timing is non-deterministic, a later event can finish logging and reach the queue before an earlier one. This violates the strict causal ordering required by the protocol and will cause downstream failures.
Consider enqueuing events immediately to preserve order, then performing logging downstream (e.g., using `Stream.tap` on the consumer side) or by serializing the ingestion path.
Evidence trail:
apps/server/src/provider/Layers/ClaudeCodeAdapter.ts lines 539-560: shows `runtimeEventQueue`, `writeNativeEvent` definition, and listener implementation using `Effect.runPromiseWith(services)` (fire-and-forget); apps/server/src/provider/Layers/EventNdjsonLogger.ts lines 228-243: shows `write` method implementation that does async file I/O via `writer.writeMessage`; apps/server/src/provider/Layers/EventNdjsonLogger.ts lines 160-171: shows `writeMessage` uses batched logging with async flush to file sink; apps/server/src/provider/Layers/CodexAdapter.ts lines 1453-1479: shows identical pattern in CodexAdapter confirming this is a consistent code pattern.
| async rollbackThread(threadId: ThreadId, numTurns: number): Promise<ClaudeCodeThreadSnapshot> { | ||
| const context = this.#getSessionContext(threadId); | ||
| if (numTurns <= 0) { | ||
| return { threadId, turns: context.turns }; | ||
| } |
There was a problem hiding this comment.
🟢 Low src/claudeCodeServerManager.ts:988
rollbackThread corrupts session state when called while a turn is active: it clears context.turns and resets resumeCursor, but leaves context.activeChild and context.activeTurn untouched. When the active process later completes, #emitTurnCompleted pushes the zombie turn into the now-cleared array, and #handleResultEvent overwrites the new resumeCursor with the old session ID from the finishing process. Consider stopping the active child and clearing the active turn state before modifying the session.
- async rollbackThread(threadId: ThreadId, numTurns: number): Promise<ClaudeCodeThreadSnapshot> {
+ async rollbackThread(threadId: ThreadId, numTurns: number): Promise<ClaudeCodeThreadSnapshot> {
const context = this.#getSessionContext(threadId);
+ if (context.activeTurn || context.activeChild) {
+ throw new Error(`Cannot rollback while session is busy: ${String(threadId)}`);
+ }
if (numTurns <= 0) {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/claudeCodeServerManager.ts around lines 988-992:
`rollbackThread` corrupts session state when called while a turn is active: it clears `context.turns` and resets `resumeCursor`, but leaves `context.activeChild` and `context.activeTurn` untouched. When the active process later completes, `#emitTurnCompleted` pushes the zombie turn into the now-cleared array, and `#handleResultEvent` overwrites the new `resumeCursor` with the old session ID from the finishing process. Consider stopping the active child and clearing the active turn state before modifying the session.
Evidence trail:
apps/server/src/claudeCodeServerManager.ts lines 988-1009 (rollbackThread method - clears turns and resumeCursor but doesn't touch activeChild/activeTurn), lines 53-56 (ClaudeSessionContext showing activeChild and activeTurn fields), lines 859-882 (#handleResultEvent - overwrites resumeCursor with session_id from finishing process), lines 461-510 (#emitTurnCompleted - pushes to context.turns)
| activeTurn: undefined, | ||
| turns: [], | ||
| }; | ||
| this.#sessions.set(input.threadId, context); |
There was a problem hiding this comment.
🟡 Medium src/claudeCodeServerManager.ts:279
startSession overwrites existing session contexts without killing the previous child process. If called for an active threadId, the orphaned process continues running and emits events tagged with that threadId, causing process leaks and event interleaving with the new session. Consider calling stopSession(threadId) before this.#sessions.set(input.threadId, context) to ensure cleanup.
};
+ this.stopSession(input.threadId);
this.#sessions.set(input.threadId, context);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/claudeCodeServerManager.ts around line 279:
`startSession` overwrites existing session contexts without killing the previous child process. If called for an active `threadId`, the orphaned process continues running and emits events tagged with that `threadId`, causing process leaks and event interleaving with the new session. Consider calling `stopSession(threadId)` before `this.#sessions.set(input.threadId, context)` to ensure cleanup.
Evidence trail:
apps/server/src/claudeCodeServerManager.ts lines 252-279 (`startSession` method creates new context and sets it without checking for existing session), line 279 (`this.#sessions.set(input.threadId, context)` overwrites existing entry), lines 959-969 (`stopSession` shows proper cleanup including `context.activeChild?.kill("SIGINT")` which is not called in `startSession`)
| const status = | ||
| lifecycle === "item.completed" | ||
| ? asString(item.status) === "failed" | ||
| ? "failed" | ||
| : "completed" | ||
| : "inProgress"; |
There was a problem hiding this comment.
🟢 Low Layers/ClaudeCodeAdapter.ts:160
mapItemLifecycle maps any status other than "failed" to "completed", so provider statuses like "cancelled" or "interrupted" are incorrectly reported as successful completions. This contradicts the handling in turn/completed which preserves these states. Consider expanding the status mapping to explicitly handle "cancelled" and "interrupted" like the turn completion logic does.
const status =
lifecycle === "item.completed"
? asString(item.status) === "failed"
? "failed"
: "completed"
: "inProgress";🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ClaudeCodeAdapter.ts around lines 160-165:
`mapItemLifecycle` maps any status other than `"failed"` to `"completed"`, so provider statuses like `"cancelled"` or `"interrupted"` are incorrectly reported as successful completions. This contradicts the handling in `turn/completed` which preserves these states. Consider expanding the status mapping to explicitly handle `"cancelled"` and `"interrupted"` like the turn completion logic does.
Evidence trail:
apps/server/src/provider/Layers/ClaudeCodeAdapter.ts lines 160-164 (mapItemLifecycle status mapping), lines 267-272 (turn/completed status mapping). Commit: REVIEWED_COMMIT.
|
Official #179 |
3 participants
Summary
Add Claude Code as a native provider using the existing provider/orchestration flow, and reconcile the branch onto latest
main.Includes
Key behavior verified
claudeCodeSearching the web,Web search complete)Verification
bun lintbun typecheckbun run testbun run --cwd apps/web test:browserbun run build:desktopAll green on
claude-pr-sync.